-
Notifications
You must be signed in to change notification settings - Fork 127
Passed Predict Params to RAPS for conformity score calculation and EnsembleClassifier #784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Hello @VishnoiAman777, thank you for the contribution! It seems that indeed
I have made comments on some of your modifications, please look at them when you have the time. Otherwise, I was wondering about the possible uses of |
|
Hi @allglc, The As you have said Additionally, I am not able to see any comments on my changes yet as you have mentioned. Have you started a code review? |
| self.y_pred_proba_raps = self.predictor.single_estimator_.predict_proba( | ||
| self.X_raps | ||
| ) | ||
| predict_params = kwargs.pop("predict_params", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you only need to add **predict_params to the function? i.e,:
self.y_pred_proba_raps = self.predictor.single_estimator_.predict_proba(self.X_raps, **predict_params) like you did in the classifier.py file.
| np.testing.assert_equal(y_pred, 0) | ||
|
|
||
|
|
||
| def test_raps_with_predict_params() -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test does not seem to test what we want. It does not test if predict params are passed into RAPS and actually this test passes on the current version of MAPIE (i.e., without your modifications).
Furthermore, the code is too complex: there is no need to recreate a dataset. Please refer to e.g., test_predict_parameters_passing for inspiration.
|
@VishnoiAman777 Thanks for the clarification! Sorry I forgot submitting the review, you should see the comments now. |
Description
This change adds a test case to ensure that
predict_paramsare correctly passed and handled when usingEnsemble Classifiertopredict_probafunction for an estimator andRAPSConformityScoreClass. Addtional testtest_raps_with_predict_paramsis added to verify that thepredict_paramsare properly passed through thefitandpredictmethods in RAPSConformityScore.Fixes #614
Type of change
How Has This Been Tested?
The following test was added to verify the changes:
test_raps_with_predict_paramspredict_paramsare correctly passed through thefitandpredictmethods when usingRAPSConformityScoreChecklist
make lintmake type-checkmake testsmake coveragemake docmake doctest